Skip to content

Conversation

@huixie90
Copy link
Member

@huixie90 huixie90 commented Jul 5, 2025

At the moment, we use the os internal functions __ulock_wait. This patch updates the code on macOS to use the public API os_sync_wait_on_address.

Fixes #146142

@huixie90 huixie90 requested a review from a team as a code owner July 5, 2025 13:28
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 5, 2025

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/147146.diff

1 Files Affected:

  • (modified) libcxx/src/atomic.cpp (+6-11)
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index c1af8d6f95aae..6cb02550be2f7 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -41,6 +41,10 @@
 // OpenBSD has no indirect syscalls
 #  define _LIBCPP_FUTEX(...) futex(__VA_ARGS__)
 
+#elif defined(__APPLE__) && defined(_LIBCPP_USE_ULOCK)
+
+#  include <os/os_sync_wait_on_address.h>
+
 #else // <- Add other operating systems here
 
 // Baseline needs no new headers
@@ -65,24 +69,15 @@ static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const vo
 
 #elif defined(__APPLE__) && defined(_LIBCPP_USE_ULOCK)
 
-extern "C" int __ulock_wait(
-    uint32_t operation, void* addr, uint64_t value, uint32_t timeout); /* timeout is specified in microseconds */
-extern "C" int __ulock_wake(uint32_t operation, void* addr, uint64_t wake_value);
-
-// https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/sys/ulock.h#L82
-#  define UL_COMPARE_AND_WAIT64 5
-#  define ULF_WAKE_ALL 0x00000100
-
 static void
 __libcpp_platform_wait_on_address(__cxx_atomic_contention_t const volatile* __ptr, __cxx_contention_t __val) {
   static_assert(sizeof(__cxx_atomic_contention_t) == 8, "Waiting on 8 bytes value");
-  __ulock_wait(UL_COMPARE_AND_WAIT64, const_cast<__cxx_atomic_contention_t*>(__ptr), __val, 0);
+  os_sync_wait_on_address(const_cast<__cxx_atomic_contention_t*>(__ptr), __val, 8, OS_SYNC_WAIT_ON_ADDRESS_NONE);
 }
 
 static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const volatile* __ptr, bool __notify_one) {
   static_assert(sizeof(__cxx_atomic_contention_t) == 8, "Waking up on 8 bytes value");
-  __ulock_wake(
-      UL_COMPARE_AND_WAIT64 | (__notify_one ? 0 : ULF_WAKE_ALL), const_cast<__cxx_atomic_contention_t*>(__ptr), 0);
+  os_sync_wake_by_address_all(const_cast<__cxx_atomic_contention_t*>(__ptr), 8, OS_SYNC_WAKE_BY_ADDRESS_NONE);
 }
 
 #elif defined(__FreeBSD__) && __SIZEOF_LONG__ == 8

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup, this is great!

@ldionne ldionne merged commit b9d8d1e into llvm:main Jul 11, 2025
76 of 79 checks passed
@JDevlieghere
Copy link
Member

This is causing a build failure on Darwin: https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/

[2025-07-11T23:04:47.605Z] /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/libcxx/src/atomic.cpp:46:12: fatal error: 'os/os_sync_wait_on_address.h' file not found
[2025-07-11T23:04:47.605Z]    46 | #  include <os/os_sync_wait_on_address.h>
[2025-07-11T23:04:47.605Z]       |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[2025-07-11T23:04:47.605Z] 1 error generated.
[2025-07-11T23:04:47.605Z] [25/486] Building CXX object libcxx/src/CMakeFiles/cxx_static.dir/atomic.cpp.o

Michael137 added a commit that referenced this pull request Jul 14, 2025
Michael137 added a commit that referenced this pull request Jul 14, 2025
Reverts #147146

This is failing to build on our public macOS CI:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-sanitized/

```
06:48:56  FAILED: libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o 
06:48:56  /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/bin/clang++ --target=arm64-apple-darwin23.1.0 -DLIBCXX_BUILDING_LIBCXXABI -DLIBC_NAMESPACE=__llvm_libc_common_utils -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/src -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/include/c++/v1 -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxxabi/include -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/cmake/Modules/../../libc -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -O3 -DNDEBUG -std=c++23 -isysroot /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk -mmacosx-version-min=14.1 -fPIC -UNDEBUG -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -fsized-deallocation -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wzero-length-array -Wdeprecated-redundant-constexpr-static-def -Wno-nullability-completeness -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -fdebug-prefix-map=/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/include/c++/v1=/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/include -MD -MT libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o -MF libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o.d -o libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o -c /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/src/atomic.cpp
06:48:56  /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/src/atomic.cpp:46:12: fatal error: 'os/os_sync_wait_on_address.h' file not found
06:48:56     46 | #  include <os/os_sync_wait_on_address.h>
06:48:56        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
06:48:56  1 error generated.
```

This is the configuration the failing bots are running:
```
06:25:12  + sw_vers
06:25:12  ProductName:		macOS
06:25:12  ProductVersion:		14.1
06:25:12  BuildVersion:		23B74
06:25:12  + xcodebuild -version
06:25:12  Xcode 15.2
06:25:12  Build version 15C5500c
```


The Intel bots are building fine though. Probably because they're on a
newer OS where the headers are available?
```
10:03:35  + sw_vers
10:03:35  ProductName:		macOS
10:03:35  ProductVersion:		15.1.1
10:03:35  BuildVersion:		24B91
10:03:35  + xcodebuild -version
10:03:39  Xcode 16.2
10:03:39  Build version 16C5031c
10:03:39  + cmake --version
10:03:39  cmake version 3.30.2
```
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 14, 2025
…" (#148705)

Reverts llvm/llvm-project#147146

This is failing to build on our public macOS CI:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-sanitized/

```
06:48:56  FAILED: libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o
06:48:56  /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/bin/clang++ --target=arm64-apple-darwin23.1.0 -DLIBCXX_BUILDING_LIBCXXABI -DLIBC_NAMESPACE=__llvm_libc_common_utils -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/src -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/include/c++/v1 -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxxabi/include -I/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/cmake/Modules/../../libc -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -O3 -DNDEBUG -std=c++23 -isysroot /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk -mmacosx-version-min=14.1 -fPIC -UNDEBUG -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -fsized-deallocation -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wzero-length-array -Wdeprecated-redundant-constexpr-static-def -Wno-nullability-completeness -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -fdebug-prefix-map=/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/lldb-build/include/c++/v1=/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/include -MD -MT libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o -MF libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o.d -o libcxx/src/CMakeFiles/cxx_shared.dir/atomic.cpp.o -c /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/src/atomic.cpp
06:48:56  /Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-sanitized/llvm-project/libcxx/src/atomic.cpp:46:12: fatal error: 'os/os_sync_wait_on_address.h' file not found
06:48:56     46 | #  include <os/os_sync_wait_on_address.h>
06:48:56        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
06:48:56  1 error generated.
```

This is the configuration the failing bots are running:
```
06:25:12  + sw_vers
06:25:12  ProductName:		macOS
06:25:12  ProductVersion:		14.1
06:25:12  BuildVersion:		23B74
06:25:12  + xcodebuild -version
06:25:12  Xcode 15.2
06:25:12  Build version 15C5500c
```

The Intel bots are building fine though. Probably because they're on a
newer OS where the headers are available?
```
10:03:35  + sw_vers
10:03:35  ProductName:		macOS
10:03:35  ProductVersion:		15.1.1
10:03:35  BuildVersion:		24B91
10:03:35  + xcodebuild -version
10:03:39  Xcode 16.2
10:03:39  Build version 16C5031c
10:03:39  + cmake --version
10:03:39  cmake version 3.30.2
```
static_assert(sizeof(__cxx_atomic_contention_t) == 8, "Waking up on 8 bytes value");
__ulock_wake(
UL_COMPARE_AND_WAIT64 | (__notify_one ? 0 : ULF_WAKE_ALL), const_cast<__cxx_atomic_contention_t*>(__ptr), 0);
os_sync_wake_by_address_all(const_cast<__cxx_atomic_contention_t*>(__ptr), 8, OS_SYNC_WAKE_BY_ADDRESS_NONE);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be calling os_sync_wake_by_address_any if __notify_one is set?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[libc++] Switch to the officially documented APIs for implementing ulock_{wait,wake} on Apple

5 participants